Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid setting out of range DAScale values #2184

Merged
merged 17 commits into from
Jan 21, 2025
Merged

Avoid setting out of range DAScale values #2184

merged 17 commits into from
Jan 21, 2025

Conversation

t-b
Copy link
Collaborator

@t-b t-b commented Jul 12, 2024 •
edited
Loading

  • Add minimum/maximum to wavebuilder stimset note
  • Fix WaveBuilder can create stimsets with NaNs/Inf inside #2180
  • Add return value to SetDAScale/SetDAScaleModOp
  • Fail with new LBN entry in this case and suggest to change "External Command Sensitivity"
    • SC_SpikeControl
    • PSQ_Chirp
    • ReachTargetVoltage
    • MSQ_FastRheoEst
    • MSQ_DAScale
    • PSQ_DAScale
    • PSQ_SquarePulse
    • PSQ_Rheobase
    • PSQ_Ramp not needed
  • Add tests
    • ReachTargetVoltage
    • SC_SpikeControl
    • MSQ_FastRheoEst
    • MSQ_DAScale
    • PSQ_Chirp
    • PSQ_DAScale
    • PSQ_SquarePulse
    • PSQ_Rheobase
    • PSQ_Ramp not needed
  • Add support in dashboard
    • ReachTargetVoltage not needed
    • SC_SpikeControl
    • MSQ_FastRheoEst
    • MSQ_DAScale
    • PSQ_Chirp
    • PSQ_DAScale
    • PSQ_SquarePulse
    • PSQ_Rheobase
    • PSQ_Ramp not needed
  • Raise versions
  • Adapt flowcharts
    • ReachTargetVoltage not needed
    • SC_SpikeControl
    • MSQ_FastRheoEst
    • MSQ_DAScale
    • PSQ_Chirp
    • PSQ_DAScale
    • PSQ_SquarePulse
    • PSQ_Rheobase
    • PSQ_Ramp not needed

Requires:

Close #2120

@t-b t-b self-assigned this Jul 12, 2024
@t-b t-b requested a review from timjarsky as a code owner July 12, 2024 20:55
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch from fdd77a9 to 4db0a55 Compare July 15, 2024 19:26
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch from 4db0a55 to 8833ec8 Compare July 31, 2024 16:14
@t-b t-b requested a review from MichaelHuth as a code owner July 31, 2024 16:14
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch 2 times, most recently from 17eb7cf to 9526a6e Compare August 6, 2024 14:04
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch 2 times, most recently from 9fdf459 to c273d9e Compare October 23, 2024 11:09
@t-b t-b mentioned this pull request Oct 25, 2024
Merged
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch from c273d9e to d405cc7 Compare October 25, 2024 19:30
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch 6 times, most recently from 1c6935c to dd12972 Compare November 5, 2024 19:46
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch 2 times, most recently from 4de9fa6 to 0690cc7 Compare November 18, 2024 13:24
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch 5 times, most recently from 1a5b7fd to 0df5774 Compare December 2, 2024 20:38
@t-b t-b assigned timjarsky and unassigned t-b Dec 3, 2024
@t-b
Copy link
Collaborator Author

t-b commented Dec 3, 2024

@timjarsky Can be reviewed once #2290 is merged.

@t-b t-b mentioned this pull request Dec 3, 2024
Draft
8 tasks
@t-b t-b mentioned this pull request Jan 6, 2025
Open
3 tasks
@timjarsky
Copy link
Collaborator

@t-b

  Some values in DataWave exceed device limits for DATA_ACQUISITION mode (channel index 1, position 3467). Maybe the DA/AD Gain needs adjustment?

With small amplitude events. See PXP.

2025_01_09_112350.zip

@timjarsky timjarsky assigned t-b and unassigned timjarsky Jan 9, 2025
@t-b
Copy link
Collaborator Author

t-b commented Jan 14, 2025 •
edited
Loading 

  Some values in DataWave exceed device limits for DATA_ACQUISITION mode (channel index 1, position 3467). Maybe the DA/AD Gain needs adjustment?

With small amplitude events. See PXP.

I see the exact same behaviour in main and also in the older 2.7 version. The scale value of 330 for DA1 seems to be too high.

@t-b t-b assigned timjarsky and unassigned t-b Jan 14, 2025
@timjarsky
Copy link
Collaborator 

  Some values in DataWave exceed device limits for DATA_ACQUISITION mode (channel index 1, position 3467). Maybe the DA/AD Gain needs adjustment?

With small amplitude events. See PXP.

I see the exact same behaviour in main and also in the older 2.7 version. The scale value of 330 for DA1 seems to be too high.

@t-b I think I was in VC when I thought I was in CC. I'll double-check.

@timjarsky timjarsky assigned t-b and unassigned timjarsky Jan 15, 2025
timjarsky
timjarsky requested changes Jan 15, 2025
View reviewed changes
Copy link
Collaborator

@timjarsky timjarsky left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Some values in DataWave exceed device limits for DATA_ACQUISITION mode (channel index 1, position 53333). Maybe the DA/AD Gain needs adjustment?

Instead of suggesting the gain adjustment, suggest a DA scale adjustment.

Feel free to merge after making the change.

t-b added 17 commits January 20, 2025 15:23
@t-b
MIES_WaveBuilder.ipf: Add per sweep minimum/maximum to wave note
f0707a8 
We need to calculate the extrema after filling them into the final stimset
to accomodate for trailing baseline which is zero.
@t-b
DAP_GetDAScaleMax: Add function to return the maximum possible DAScal…
f180ef0 
…e value
@t-b
SetDaScale/SetDAScaleModOp: Allow to respect DAScale limits
e3edfe8 
When limitCheck is true we now return 1 when setting the new DAScale value
would be out of range for the current external command sensitivity of the
MCC amplifier.

This also requires that we now pass in the sweep number.

The default of limitCheck is still false. Only when all analysis functions
are ported, can we flip the default.
@t-b
MIES_AnalysisFunctions.ipf: Add CreateOverrideResults
80abdda
@t-b
ReportOutOfRangeDAScale: Add it
0ba3c50
@t-b
ReachTargetVoltage: Port to new SetDAscale behaviour
9d49cd0 
We can't check the limits in the PRE_DAQ_EVENT event, so we have to
disable it there.
@t-b
SC_SpikeControl: Add support for DAScale limit checks
628c1ea
@t-b
PSQ_Ramp: Add support for DAScale limit checks
48ec217
@t-b
MSQ_FastRheoEst: Add support for DAScale limit checks
4c00f3f
@t-b
MSQ_DAScale: Add support for DAScale limit checks
d93abb8
@t-b
PSQ_DAScale: Add support for DAScale limit checks
5d5d85c
@t-b
PSQ_SquarePulse: Add support for DAScale limit checks
513bbf6
@t-b
PSQ_Rheobase: Add support for DAScale limit checks
55a7146
@t-b
PSQ_Chirp: Add support for DAScale limit checks
ed7210f
@t-b
MIES_Constants.ipf: Raise versions
468ef41
@t-b
Packages/doc/dot: Adapt flowcharts for out of range DAScale values
15ef1de
@t-b
DC_PlaceDataInDAQDataWave: Suggest to tweak DAScale in case of too la…
db6e32c 
…rge values

Change requested by Tim Jarsky.
@t-b t-b force-pushed the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch from 0df5774 to db6e32c Compare January 20, 2025 14:42
@t-b
Copy link
Collaborator Author

t-b commented Jan 20, 2025

Will merge once CI passes per #2184 (review).

@t-b t-b merged commit be804f4 into main Jan 21, 2025
20 checks passed
@t-b t-b deleted the feature/2184-avoid-setting-out-of-scale-stim-scale-factor branch January 21, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timjarsky timjarsky timjarsky requested changes

@MichaelHuth MichaelHuth Awaiting requested review from MichaelHuth MichaelHuth is a code owner

Assignees

@t-b t-b

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@t-b @timjarsky